Stop enable_padding_free_metadata writing seq_lengths into the caller's examples - #8049

You are right, and the reason is bigger than a nullable column, so thanks for pushing on it. Fixed in 00450464b.

I had framed the write-back as caching, which was wrong. It was load-bearing. TRL's DataCollatorForLanguageModeling decides whether to use seq_lengths at all from the first row (sft_trainer.py line 464: [example["seq_lengths"] for example in examples] if "seq_lengths" in examples[0] else None) and only then reads it off every row, so removing the key changes what it builds.

Two things break, not one. Your null case reaches sum(None). The one I had not seen is a batch mixing an unpacked row with a packed one, where the key is missing on row 0 so TRL ignores seq_lengths entirely and falls back to torch.arange:

with derived lengths : [0, 1, 2, 0, 1, 0, 1]
key missing on row 0 : [0, 1, 2, 0, 1, 2, 3]

The packed row's second document starts at position 2 instead of 0, so it attends across the document boundary. That is silent, and it is exactly what padding-free metadata exists to prevent.

Your suggested shape is what I did: pass a normalized copy to the wrapped collator, built only when something was actually derived, so the common path still hands the caller's own list straight through with no per-row copies. Ran it three ways against upstream/main, this branch before the change and after: main passes everything except leaving seq_lengths on the caller's rows, the previous commit fixes that and breaks the null case and the mixed batch, and the new commit passes all five. The test now asserts what reaches the collator rather than only what comes back.

Review record for #8049, carried over from the mirror it was reviewed on: danielhanchen/unsloth-staging-review#309.

Findings are quoted as posted and attributed to the account that posted them. The reactions shown are the triage recorded on the mirror; none were re-applied here. Commits are named as the mirror's, with this PR's equivalent where one was found.

Round 1: reviewed mirror 7cca354cbe (this PR's a6b907f34)

chatgpt-codex-connector[bot] (on the mirror)

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 7cca354cbe

No triage reaction was recorded on the mirror

Verdict

The round converged at mirror 7cca354cbe (this PR's a6b907f34).

This is the review as it stood at that commit. Anything pushed to this PR afterwards was not part of it.